Skip to content

Tighten auth failure lockout threshold#1411

Closed
massx1 wants to merge 1 commit into
apache:masterfrom
massx1:security-auth-bruteforce-lockout
Closed

Tighten auth failure lockout threshold#1411
massx1 wants to merge 1 commit into
apache:masterfrom
massx1:security-auth-bruteforce-lockout

Conversation

@massx1
Copy link
Copy Markdown
Contributor

@massx1 massx1 commented Jun 4, 2026

Before this change, the account policy threshold was enforced on attempt N + 1: with maxAuthenticationAttempts = 3, the user was still active after 3 failures and suspended only on the 4th failure.

The old test encoded this by asserting failedLogins == 3 before executing one more failed login.

@massx1 massx1 force-pushed the security-auth-bruteforce-lockout branch from 8586ea8 to 7cd8f13 Compare June 4, 2026 08:51
assertEquals(3, USER_SERVICE.read(userKey).getFailedLogins());
userTO = USER_SERVICE.read(userTO.getKey());
assertEquals(2, userTO.getFailedLogins().intValue());
assertNotEquals("suspended", userTO.getStatus());
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The status value comes from the workflow definition in use, so it might vary.

The cleanest check here is

assertFalse(userTO.isSuspended();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I have to do the same thing few rows below adding assertTrue(userTO.isSuspended());

userTO = USER_SERVICE.read(userTO.getKey());
assertNotNull(userTO);
assertNotNull(userTO.getFailedLogins());
assertEquals(3, userTO.getFailedLogins().intValue());
assertEquals("suspended", userTO.getStatus());

@ilgrosso
Copy link
Copy Markdown
Member

ilgrosso commented Jun 4, 2026

Before this change, the account policy threshold was enforced on attempt N + 1: with maxAuthenticationAttempts = 3, the user was still active after 3 failures and suspended only on the 4th failure.

maxAuthenticationAttempts = 3

means that the number of authentication attempts allowed to fail is 3, at maximum.

So the fact that "the user was still active after 3 failures and suspended only on the 4th failure" is on purpose, not an error.

@ilgrosso ilgrosso closed this Jun 4, 2026
@ilgrosso
Copy link
Copy Markdown
Member

ilgrosso commented Jun 4, 2026

@massx1 please reach out to dev@ to discuss, when you are proposing such changes, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants